Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes for isLegalPrefix change #22241

Merged
merged 4 commits into from
Jan 13, 2025

Conversation

dwijnand
Copy link
Member

Fixes #22062
Fixes #22068
Fixes #22070

@dwijnand dwijnand marked this pull request as ready for review December 18, 2024 18:53
@dwijnand dwijnand requested a review from odersky December 18, 2024 18:53
Copy link
Contributor

@odersky odersky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will hopefully fix more issues. I particularly like how the OpaqueInlineProxy was factored out into its own abstraction.

Except for one comment LGTM

@@ -768,6 +790,7 @@ class Inliner(val call: tpd.Tree)(using Context):
override def typedSelect(tree: untpd.Select, pt: Type)(using Context): Tree = {
val locked = ctx.typerState.ownedVars
val qual1 = typed(tree.qualifier, shallowSelectionProto(tree.name, pt, this, tree.nameSpan))
selectionType(tree, qual1) // side-effect
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to be more specific what side effect (error messages?, anything else?)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It fixes the denotation of the hash-consed TermRef. So in Featureful[?]#toFeatures, selectionType will skolemize the prefix, find the denotation, and then set that denotation for the TermRef(Featureful[?], symbol toFeatures).

@odersky odersky assigned dwijnand and unassigned odersky Jan 11, 2025
@odersky
Copy link
Contributor

odersky commented Jan 11, 2025

This should get in before the cutoff.

@WojciechMazur
Copy link
Contributor

Is there anything left to do with this changes? Would be great if we could confirm the regressions are fixed before the cutoff

@odersky
Copy link
Contributor

odersky commented Jan 13, 2025

It was just an open question. We can merge and get an answer later.

@odersky odersky merged commit a5a9fc8 into scala:main Jan 13, 2025
29 checks passed
@dwijnand dwijnand deleted the i22062-reg-from-isLegalPrefix branch January 13, 2025 11:46
@dwijnand
Copy link
Member Author

#22358

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants